Add long URLs to Most Visited table on homepage #127
Conversation
Signed-off-by: shaftoes <[email protected]>
Signed-off-by: shaftoes <[email protected]>
shaftoes
force-pushed
the
add-full-url-to-homepage
branch
from
ba76ed2 to
877c565
Compare
|
@willnorris any chance for a review on this? |
| @@ -404,10 +405,21 @@ func serveHandler() http.Handler { | |||
| func serveHome(w http.ResponseWriter, r *http.Request, short string) { | |||
| var clicks []visitData | |||
|
|
|||
| linkMap := map[string]string{} | |||
| links, err := db.LoadAll() | |||
There was a problem hiding this comment.
db.LoadAll() here sort of defeats the use of the in memory stats struct to keep the initial page home page load performant if the db has many links in it.
Instead of loading everything and mapping it against the in memory variable, could the stats struct instead be updated to include the long URL also for display?
It would keep the main page snappy and avoids hitting the db/disk if there's many links to load.
There was a problem hiding this comment.
sorry about the delay. my review was mainly going to focus on this. Loading every link on every page load isn't great. The issue with storing the long URL in the stats struct is just needing to make sure to keep it up to date with changes to the link value.
As for rendering, we should use the same style as the http://go/.all page (long URL underneath the short URL) rather than another column to the right.
We've been using golinks internally for a few months now and are really loving it!
As it stands, the Most Visited table is a great place for people to find useful shortlinks, but if the name isn't descriptive then your only recourse is to click the link. This makes it harder for new users to start getting value out of the service.